fix: omit nonResourceURLs from namespaced Roles - #1278
Conversation
Signed-off-by: Alka Kumari <alkumari@redhat.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe controller now filters non-resource rules before aggregating ClusterRole rules into the application-controller Role. Unit and end-to-end tests validate resource rule preservation and non-resource rule omission. ChangesNamespaced Role filtering
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Namespaced application-controller Roles now omit invalid non-resource URL permissions while retaining resource permissions, preventing reconciliation rejection without an identified remaining merge risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
|
/retest |
3 participants
What type of PR is this?
/kind bug
What does this PR do / why we need it:
The application-controller Role copies rules from the aggregated admin ClusterRole. Third-party ClusterRoles labeled aggregate-to-admin can contribute nonResourceURLs, which are illegal on a Role, so the API server rejects it and Argo CD reconcile hard-fails (including Dex / OpenShift OAuth).
This PR fixes that by stripping nonResourceURLs from the namespace role while copying from admin ClusterRole.
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
argoproj-labs/argocd-operator#2269
https://redhat.atlassian.net/browse/GITOPS-10618
Test acceptance criteria:
How to test changes / Special notes to the reviewer:
Steps to Reproduce
Install the affected version of the OpenShift GitOps Operator 1.19.3.
Create a ClusterRole with the aggregate-to-admin label and a nonResourceURLs rule:
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: test-aggregated-admin-role
labels:
rbac.authorization.k8s.io/aggregate-to-admin: "true"
rules:
verbs:
Apply the ClusterRole:
oc apply -f test-aggregated-admin-role.yaml
Verify that the nonResourceURLs rule is propagated to the aggregated admin ClusterRole:
oc get clusterrole admin -o yaml
Create or reconcile an Argo CD instance managed by the OpenShift GitOps Operator.
Observe that the GitOps Operator attempts to create/update the namespaced Role:
openshift-gitops-argocd-application-controller
The reconciliation fails with:
Role.rbac.authorization.k8s.io "openshift-gitops-argocd-application-controller" is invalid:
rules[814].nonResourceURLs: Invalid value: []string{"*"}:
namespaced rules cannot apply to non-resource URLs
Expected Result
The GitOps Operator should not copy the nonResourceURLs rule into the namespaced Application Controller Role. Argo CD reconciliation should complete successfully.